Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Queues - Marisol Lopez - ride_share.rb #36

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

marisol-lopez
Copy link

Ride Share

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a design decision you had to make when working on this project. What options were you considering? What helped you make your final decision? Initially I thought about linking the three separate class files by requiring each class at the top, but when I needed to write tests for classes that were called within another class I couldn't do it, so I wrapped each class in a module.
Describe a concept that you gained more clarity on as you worked on this assignment. encapsulation.
Describe a nominal test that you wrote for this assignment. I wrote one to check to make sure that the vin had to be 17 characters.
Describe an edge case test that you wrote for this assignment. I don't think I did to be honest. I don't really understand edge casing.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I felt great mapping out a lot of my stuff first, and when I was trying to write a method, I would write out what I wanted the method to do, and that would help me write my tests.

@PilgrimMemoirs
Copy link

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly Need to commit more regularly, with more specific messages
Answer comprehension questions Mostly Good - would like more of an explanation on how you're better understanding encapsulation with this project.
Driver
Uses the all method in the find method Well Done
Has appropriate edge-case tests for each method in the class Some - could use more
Created a method that uses a method from the Trip object to retrieve the list of trips Well Done
Created a method that uses the internal trips list to calculate the average rating ❗️ Instead of having 'RideShare::Trip.find_trips_of_driver(id)' inside of the rating method, it should call 'trip_instances_for_driver'. Why would that be better?
Rider
Uses the all method in the find method Well Done
Has appropriate edge-case tests for each method in the class Mostly good
Created a method that uses a method from the Trip object to retrieve the list of trips Well Done
Created a method that uses the internal trips method to retrieve the associated drivers ❗️ Did not do - same idea as with driver's trips.
Trip
Reads the CSV file in the all method
Has appropriate edge-case tests for each method in the class Mostly good
Created a method that uses a method from the Driver to retrieve the associated driver instance Well Done
Created a method that uses a method from the Rider to retrieve the associated rider instance Well Done
Created a method to retrieve all trips by driver id ❗️ Missing
Created a method to retrieve all trips by rider id ❗️ Missing
Overall
Needs missing methods added - everything else looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants